Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: improve 1D line broadening filter #3260

Merged
merged 42 commits into from
Nov 20, 2024
Merged

feat: improve 1D line broadening filter #3260

merged 42 commits into from
Nov 20, 2024

Conversation

jobo322
Copy link
Member

@jobo322 jobo322 commented Oct 7, 2024 •
edited by hamed-musallam
Loading

@jobo322 jobo322 linked an issue Oct 7, 2024 that may be closed by this pull request
crash, at the moment to show peak shapes of peaks from manual peak-picking #2931
Closed
@jobo322 jobo322 changed the title 2931 crash at the moment to show peak shapes of peaks from manual peak picking add shape property to peaks added manually Oct 7, 2024
@cloudflare-workers-and-pages Cloudflare Workers and Pages
Copy link

cloudflare-workers-and-pages bot commented Oct 7, 2024 •
edited
Loading

Deploying nmrium with  Cloudflare Pages  Cloudflare Pages

Latest commit: 0da03c7
Status: ✅  Deploy successful!
Preview URL: https://5b1dec99.nmrium.pages.dev
Branch Preview URL: https://2931-crash-at-the-moment-to.nmrium.pages.dev

View logs

@jobo322 jobo322 force-pushed the 2931-crash-at-the-moment-to-show-peak-shapes-of-peaks-from-manual-peak-picking branch 2 times, most recently from c829aa4 to 39978a8 Compare October 13, 2024 04:51
@hamed-musallam hamed-musallam force-pushed the 2931-crash-at-the-moment-to-show-peak-shapes-of-peaks-from-manual-peak-picking branch from 0f22b19 to 3095c77 Compare October 14, 2024 14:02
@hamed-musallam hamed-musallam changed the title add shape property to peaks added manually feat: improve 1D line broadening filter Oct 14, 2024
This was linked to issues Oct 22, 2024
Migration of 1D apodization filter #3266
Closed
exported complex 1D jcamp from nmrium can not be loaded back #3267
Closed
Change of filter format towards 2D processing #3261
Closed
@lpatiny
Copy link
Member

lpatiny commented Oct 22, 2024

First conflict should be solved and then I don't know which issue is solved with this PR.

@hamed-musallam hamed-musallam force-pushed the 2931-crash-at-the-moment-to-show-peak-shapes-of-peaks-from-manual-peak-picking branch 2 times, most recently from d9bf3f0 to 8984793 Compare October 23, 2024 11:38
lpatiny
lpatiny reviewed Oct 31, 2024
View reviewed changes
package.json Outdated Show resolved Hide resolved
@jobo322
Copy link
Member Author

jobo322 commented Nov 4, 2024

@targos @lpatiny, optional parameters does not exist anymore in filters. In nmrium there is a conflict in the onLoadProcessing:

-https://github.com/cheminfo/nmrium/blob/main/src/demo/views/SnapshotView.tsx#L428-L433

I propose do not use the same type of Filters, then at the moment to apply the filters we could completed the options needed

@lpatiny
Copy link
Member

lpatiny commented Nov 10, 2024

@jobo322 Did you check why the testcases are not passing ?

@hamed-musallam hamed-musallam force-pushed the 2931-crash-at-the-moment-to-show-peak-shapes-of-peaks-from-manual-peak-picking branch from c57ea9f to 5df3d47 Compare November 15, 2024 11:19
@hamed-musallam hamed-musallam force-pushed the 2931-crash-at-the-moment-to-show-peak-shapes-of-peaks-from-manual-peak-picking branch from 34b9889 to 41c44c4 Compare November 19, 2024 09:51
@lpatiny
Copy link
Member

lpatiny commented Nov 19, 2024

@targos We may need your help to debug the lint-check-types problem.

We have the following error is we run npx tsc --noEmit --traceResolution

======== Resolving module '../../../data/utilities/getAtomsFromMF.js' from '/Users/lpatiny/git/cheminfo/nmrium/src/component/panels/SummaryPanel/SetMolecularFormulaModal.tsx'. ========
Module resolution kind is not specified, using 'NodeNext'.
Resolving in ESM mode with conditions 'import', 'types', 'node'.
Loading module as file / folder, candidate module location '/Users/lpatiny/git/cheminfo/nmrium/src/data/utilities/getAtomsFromMF.js', target file types: TypeScript, JavaScript, Declaration, JSON.
File name '/Users/lpatiny/git/cheminfo/nmrium/src/data/utilities/getAtomsFromMF.js' has a '.js' extension - stripping it.
File '/Users/lpatiny/git/cheminfo/nmrium/src/data/utilities/getAtomsFromMF.ts' exists - use it as a name resolution result.
======== Module name '../../../data/utilities/getAtomsFromMF.js' was successfully resolved to '/Users/lpatiny/git/cheminfo/nmrium/src/data/utilities/getAtomsFromMF.ts'. ========
/Users/lpatiny/git/cheminfo/nmrium/node_modules/typescript/lib/tsc.js:120986
      throw e;
      ^

RangeError: Map maximum size exceeded
    at Map.set (<anonymous>)
    at checkTypeRelatedTo (/Users/lpatiny/git/cheminfo/nmrium/node_modules/typescript/lib/tsc.js:63276:16)
    at isTypeRelatedTo (/Users/lpatiny/git/cheminfo/nmrium/node_modules/typescript/lib/tsc.js:63188:14)
    at isTypeStrictSubtypeOf (/Users/lpatiny/git/cheminfo/nmrium/node_modules/typescript/lib/tsc.js:62312:12)

@jobo322
Copy link
Member Author

jobo322 commented Nov 19, 2024

At the moment open the general settings it throws an error:

image

@jobo322
Copy link
Member Author

jobo322 commented Nov 19, 2024

The filters in the processings panel can't be removed.

@hamed-musallam hamed-musallam force-pushed the 2931-crash-at-the-moment-to-show-peak-shapes-of-peaks-from-manual-peak-picking branch from 36a5b1e to 0da03c7 Compare November 20, 2024 18:42
@hamed-musallam
Copy link
Member

Zones panel preferences are not working anymore

@hamed-musallam hamed-musallam mentioned this pull request Nov 20, 2024
Closed
@lpatiny lpatiny merged commit 797fea4 into main Nov 20, 2024
13 checks passed
@lpatiny lpatiny deleted the 2931-crash-at-the-moment-to-show-peak-shapes-of-peaks-from-manual-peak-picking branch November 20, 2024 21:14
@cheminfo-bot cheminfo-bot mentioned this pull request Nov 20, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lpatiny lpatiny lpatiny left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@jobo322 @lpatiny @hamed-musallam @targos